Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies to match gulp 4 #108

Merged
merged 4 commits into from
May 9, 2016
Merged

Update dependencies to match gulp 4 #108

merged 4 commits into from
May 9, 2016

Conversation

doowb
Copy link
Contributor

@doowb doowb commented Apr 28, 2016

This PR is to update dependencies to match some that are used in gulp 4.

  • removes gulp-util
  • uses fancy-log for DEBUG logging
  • updates findup-sync
  • uses micromatch (used in findup-sync and chokidar for gulp.watch)

Since this is an attempt at consolidating common dependencies, I removed the green coloring around the logging messages. If you'd like to keep that, I can add something for it.

@callumacrae
Copy link
Contributor

Maybe worth using gulplog?

Nice work, though :)

@jackfranklin
Copy link
Owner

Agree with @callumacrae that I'd use gulplog but once that's done awesome work, thank you!

Can you add yourself as a contributor in the package.json too please? :)

@tunnckoCore
Copy link

tunnckoCore commented Apr 28, 2016

I think it worth using https://github.com/tunnckoCore/load-deps under the hoods, it's pretty fast. Or just take a look and say some feedback :)

Cheers,
Charlike!

@jackfranklin
Copy link
Owner

@tunnckoCore can you open a new issue? It's not related to this PR.

adding myself to contributors list
@doowb
Copy link
Contributor Author

doowb commented Apr 28, 2016

I updated to use gulplog but there are a few caveats:

  • gulplog only emits an event so it requires that users are using gulp-cli which has the event handling configured.
  • gulp-cli logs events based on the -L flag level so I set the logger to do logger.debug which requires the -LLLL to be used.
  • Changed the DEBUG option to default to true when undefined. This will still only log when -LLLL is used but it allows users to disable it through the DEBUG option by passing in { DEBUG: false }.

For the first bullet, we could listen for the events and write to the console but this will result in duplicate logging when users are using gulp-cli.

Also, added my name to the contributors list, thanks!

pattern: ['gulp-*', 'gulp.*'], // the glob(s) to search for
config: 'package.json', // where to find the plugins, by default searched up from process.cwd()
scope: ['dependencies', 'devDependencies', 'peerDependencies'], // which keys in the config to look within
replaceString: /^gulp(-|\.)/, // what to remove from the name of the module when adding it to the context
camelize: true, // if true, transforms hyphenated plugins names to camel case
lazy: true, // whether the plugins should be lazy loaded on demand
rename: {}, // a mapping of plugins to rename
renameFn: function (name) { ... } // a function to handle the renaming of plugins (the default works)
renameFn: function (name) { ... }, // a function to handle the renaming of plugins (the default works)
DEBUG: true // when set to true and using gulp-cli flag -LLLL, the plugin will log info to console. Useful for bug reporting and issue debugging
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep this as false, as this shows the default values and DEBUG should be false by default.

@doowb
Copy link
Contributor Author

doowb commented May 3, 2016

@jackfranklin I reverted the DEBUG changes per your inline comments.

@jackfranklin
Copy link
Owner

Thank you! @callumacrae does this look good to you? :)

@callumacrae
Copy link
Contributor

Yep, looks good!

@jackfranklin jackfranklin merged commit 1c080cd into jackfranklin:master May 9, 2016
@doowb
Copy link
Contributor Author

doowb commented May 19, 2016

@jackfranklin thanks for merging. Will you publish to npm?

@jackfranklin
Copy link
Owner

@doowb released as 1.2.3. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants